feat: make all internal rate limits configurable #5095
Conversation
In addition this PR exposes the limits set to prometheus under the
rate_limit{endpoint, method} gauge.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Ignored Deployment
|
| @@ -19,6 +19,7 @@ import { | |||
| ICspDomainOptions, | |||
| IClientCachingOption, | |||
| IMetricsRateLimiting, | |||
| IRateLimiting, | |||
There was a problem hiding this comment.
indeed, I was quite IRate while writing this :P
| process.env.CREATE_USER_RATE_LIMIT_PER_MINUTE, | ||
| 20, | ||
| ); | ||
| const simpleLoginMaxPerMinute = parseEnvVarNumber( | ||
| process.env.SIMPLE_LOGIN_LIMIT_PER_MINUTE, | ||
| 10, |
There was a problem hiding this comment.
Curious as to why one is called "x_RATE_LIMIT_PER_MINUTE" and the other is just called "x_LIMIT_PER_MINUTE"? Not a big deal, but feels like they should have similar names to make it easier to remember.
| @@ -259,6 +265,42 @@ export default class MetricsMonitor { | |||
| .labels({ range: clientStat.range }) | |||
| .set(clientStat.count), | |||
| ); | |||
|
|
|||
| rateLimits.reset(); | |||
There was a problem hiding this comment.
Any particular reason we need to reset it here? Will we have gotten any inputs before this point that we need to get rid of?
There was a problem hiding this comment.
I'm still not sure, I just followed the patterns for the rest of our static metrics.
There was a problem hiding this comment.
In my mind since it's a gauge there shouldn't really be a need to reset it.
| rateLimits | ||
| .labels({ endpoint: '/api/client/metrics', method: 'POST' }) | ||
| .set(config.metricsRateLimiting.clientMetricsMaxPerMinute); | ||
| rateLimits | ||
| .labels({ | ||
| endpoint: '/api/client/register', | ||
| method: 'POST', | ||
| }) | ||
| .set(config.metricsRateLimiting.clientRegisterMaxPerMinute); | ||
| rateLimits | ||
| .labels({ | ||
| endpoint: '/api/frontend/metrics', | ||
| method: 'POST', | ||
| }) | ||
| .set( | ||
| config.metricsRateLimiting.frontendMetricsMaxPerMinute, | ||
| ); | ||
| rateLimits | ||
| .labels({ | ||
| endpoint: '/api/frontend/register', | ||
| method: 'POST', | ||
| }) | ||
| .set( | ||
| config.metricsRateLimiting.frontendRegisterMaxPerMinute, | ||
| ); |
There was a problem hiding this comment.
Out of curiosity: what were all of these before? If these options are old, how were they applied? Did we do it somewhere else?
There was a problem hiding this comment.
there were no limits for register and metrics earlier besides what's configured on the loadbalancer side.
For the user creation and login it was 10 and 20.
What
This PR makes the rate limit for user creation and simple login (our password based login) configurable in the same way you can do metricsRateLimiting.
Worth noting
In addition this PR adds a
rate_limit{endpoint, method}prometheus gauge, which gets the data from the UnleashConfig.